refactor!: use Celestia node fees estimation system - #2822

The latest Buf updates on your PR. Results from workflow CI and Release / buf-check (pull_request).

Claude finished @randygrok's task —— View job

Code Review Complete

I've performed a comprehensive review of this PR refactoring the gas price/multiplier handling. Overall, this is a well-executed breaking change that significantly simplifies the DA interface and centralizes gas configuration management. Here are my findings:

Summary

This PR successfully removes GasPrice() and GasMultiplier() methods from the DA interface, moving gas configuration entirely to application config files. This allows celestia-node to handle fee estimation natively, which is the correct architectural decision. The refactoring is thorough and consistent across 39 files with a net reduction of 220 lines of code.

Strengths

1. Clean Interface Simplification: Removing gas price/multiplier methods from the DA interface is architecturally sound - gas configuration belongs in application config, not in the DA abstraction layer.

2. Thorough Implementation: All affected components have been properly updated:

   • Core DA interface and implementations (DummyDA, LocalDA)
   • JSON-RPC client/server
   • DA submitter retry logic (simplified significantly)
   • Configuration and CLI flags (properly removed)
   • All test suites updated consistently
   • Documentation (CHANGELOG.md properly updated)

3. Improved Retry Logic: The new retry policy is cleaner:

   • Removed complex gas price escalation
   • Simplified retryPolicy to focus on attempt/backoff bounds only
   • retryState no longer tracks gas price
   • Better separation of concerns

4. Good Test Coverage: Tests properly validate the new behavior, especially da_retry_test.go which covers all retry state transitions without gas price logic.

5. Backward Compatibility Handled: The go.mod replace directives ensure proper local dependency resolution during development.

Issues & Recommendations

1. Gemini's Unused Context Concern ✅

Status: Already addressed in the code.

Gemini Code Assist flagged two functions (getGasMultiplier and initialGasPrice) as having unused ctx parameters. I verified these functions were completely removed in this PR rather than having dangling parameters, so this concern is moot. Good job on the thorough cleanup!

2. Gas Price Still Passed to DA Layer (Low Priority - By Design)

Files:

• block/internal/submitting/da_submitter.go:414
• da/jsonrpc/client.go:123-136
• da/jsonrpc/server.go:64-73
• core/da/da.go:31-34

The DA interface still includes gasPrice parameters in Submit and SubmitWithOptions:

Submit(ctx context.Context, blobs []Blob, gasPrice float64, namespace []byte) ([]ID, error)
SubmitWithOptions(ctx context.Context, blobs []Blob, gasPrice float64, namespace []byte, options []byte) ([]ID, error)

However, in submitToDA (line 414), the gas price is hardcoded to -1:

res := types.SubmitWithHelpers(submitCtx, s.da, s.logger, marshaled, -1, namespace, mergedOptions)

Analysis: This appears intentional - the -1 acts as a sentinel value indicating "use DA layer's native fee estimation" (i.e., celestia-node's fee system). This is consistent with the PR's goal. However, this could be clarified:

Recommendations:

• Consider adding a constant like UseNativeFeeEstimation = -1 to make the intent explicit
• Add a comment at the call site explaining why -1 is passed
• Future consideration: Could potentially remove gasPrice parameter entirely from DA interface if it's always -1, but this might break compatibility with other DA layers

3. Configuration Validation ✅

File: pkg/config/config.go:156-167

Good to see that GasPrice and GasMultiplier fields were completely removed from DAConfig. The configuration validation is thorough, including namespace validation and lazy mode checks.

4. Mock Generation Consistency ⚠️

Files: block/internal/submitting/da_submitter_mocks_test.go

The mock generation was updated to remove gas price methods. Ensure that:

• make mock-gen has been run to regenerate all mocks
• No stale mocks remain in the codebase

Verification needed: Run make mock-gen to ensure all mocks are current.

5. JSON-RPC Client MaxBlobSize Handling ✅

File: da/jsonrpc/client.go:209-234

The client properly accepts and stores maxBlobSize during initialization. The SubmitWithOptions method validates blob sizes before submission (lines 141-162), which is good defensive programming.

6. Test Coverage for New Behavior ✅

Files:

• block/internal/submitting/da_retry_test.go - Excellent table-driven tests for retry state machine
• block/internal/submitting/da_submitter_test.go - Updated appropriately
• Mock tests updated consistently

The test refactoring properly removes gas price/multiplier logic while maintaining coverage of retry behavior, batch sizing, and error handling.

7. Migration Path / Breaking Change Communication ⚠️

File: CHANGELOG.md:18

The changelog entry is present but minimal:

- Remove GasPrice and GasMultiplier from DA interface and configuration to use celestia-node's native fee estimation. ([#2822](https://github.com/evstack/ev-node/pull/2822))

Recommendation: Enhance the changelog to guide users:

### Changed

- **BREAKING:** Remove GasPrice and GasMultiplier from DA interface and configuration to use celestia-node's native fee estimation. ([#2822](https://github.com/evstack/ev-node/pull/2822))
  - Removed CLI flags: `--evnode.da.gas_price`, `--evnode.da.gas_multiplier`
  - Removed config fields: `da.gas_price`, `da.gas_multiplier`
  - Gas fees are now handled natively by the underlying DA layer (e.g., celestia-node)
  - **Migration**: Remove these fields from your configuration files. Fee estimation will now be automatic.

Security Considerations ✅

1. No Credential Exposure: The refactoring doesn't introduce any security risks related to secrets or credentials.
2. Input Validation: Namespace validation is properly maintained (validateNamespace functions).
3. Size Limits: Blob size validation is preserved and correctly enforced before submission.
4. Concurrency Safety: No new race conditions introduced; mutex usage remains sound.

Performance Considerations ✅

1. Reduced Complexity: Removing gas price retrieval and escalation logic reduces computational overhead in retry loops.
2. No Blocking Operations Added: The refactoring doesn't introduce any new blocking calls.
3. Memory Efficiency: Net reduction of code suggests improved memory footprint.

Code Quality ✅

1. Error Handling: Consistent and proper throughout the changes.
2. Logging: Appropriate log levels maintained; gas price logging removed where applicable.
3. Documentation: Inline comments are clear; function documentation is maintained.
4. Naming Conventions: Go idioms followed consistently.

CI Status

At review time, CI is still running:

• Semantic Pull Request: ✅ Passed
• CI: 🔄 In Progress
• Buf Check: 🔄 In Progress

Previous reviewer (alpe) noted "CI is failing though" - recommend checking CI logs once complete to address any issues.

Final Verdict

Approval Status: LGTM with minor suggestions ✅

This is a high-quality refactoring that achieves its stated goals:

• ✅ Removes gas price/multiplier from DA interface
• ✅ Simplifies retry logic significantly
• ✅ Updates all affected code paths consistently
• ✅ Maintains test coverage
• ✅ Documents breaking changes

Minor Improvements:

1. Enhance CHANGELOG with migration guidance (low priority)
2. Consider adding a constant for the -1 sentinel value (optional)
3. Verify make mock-gen has been run (verification step)
4. Monitor CI completion to address any failures

Recommendation: This PR is ready to merge once CI passes. The breaking change is well-justified and properly implemented.

Summary of Changes

Hello @randygrok, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the Data Availability (DA) layer's gas configuration mechanism. The primary goal is to simplify the DA interface and ensure compatibility with external systems like celestia-node by centralizing gas price and multiplier settings within application configuration files. This change removes the responsibility of providing gas parameters from the DA interface itself, leading to a more predictable and manageable gas parameter flow throughout the system.

Highlights

• DA Interface Simplification: The GasPrice() and GasMultiplier() methods have been removed from the core DA interface, streamlining its definition and reducing its responsibilities.
• Centralized Gas Configuration: Gas-related parameters (GasPrice and GasMultiplier) are now exclusively managed through configuration files, eliminating their dynamic retrieval from the DA layer and ensuring a single source of truth.
• Broad Codebase Adaptation: All relevant components, including DummyDA, LocalDA, JSON-RPC client/server, the DA Submitter, various applications (evm/single, grpc/single, testapp), and their respective tests, have been updated to reflect this new configuration approach.
• Dependency Management Updates: Added replace directives in go.mod files across several modules to ensure proper local dependency resolution for core and da packages during development.

Codecov Report

❌ Patch coverage is 92.30769% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.65%. Comparing base (3a9c9ba) to head (0f649e2).
⚠️ Report is 1 commits behind head on main.

@@            Coverage Diff             @@
##             main    #2822      +/-   ##
==========================================
- Coverage   64.79%   64.65%   -0.14%     
==========================================
  Files          81       81              
  Lines        7224     7170      -54     
==========================================
- Hits         4681     4636      -45     
+ Misses       2003     1994       -9     
  Partials      540      540              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).